-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
meta: update module pages in CODEOWNERS #34932
Conversation
Review requested:
|
This will be blocked until that lands as the linting for CODEOWNERS will fail for non-existent paths. |
This is a good idea once we have all files listed (we're still very far today) |
@richardlau #34748 has landed, is there something I can do to unblock this PR? |
Rebase so that it’s picked up — that should get the lint-codeowners check to pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Landed in 622ea75 |
PR-URL: #34932 Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Derek Lewis <[email protected]> Reviewed-By: Myles Borins <[email protected]>
#34875 is not getting much love because @nodejs-github-bot didn't know who to tag. Can someone ping @nodejs/modules over there please? |
This depends on #34748 (otherwise the CODEOWNER linting breaks) and that has the |
PR-URL: #34932 Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Derek Lewis <[email protected]> Reviewed-By: Myles Borins <[email protected]>
PR-URL: #34932 Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Derek Lewis <[email protected]> Reviewed-By: Myles Borins <[email protected]>
I figured that should explain why the bot didn't tag @nodejs/modules in #34875.
Maybe we could add a test to check if every file is listed to the CODEOWNERS to avoid it happening again?
I've also added
/doc/api/packages.md
, which will be introduced by #34748, it's probably a good idea to wait for it to land first before landing this PR.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes